feat(buildrunner/noop): add pass-through BuildRunner stub#179
Merged
Conversation
This was referenced Jun 2, 2026
50881dd to
a7226e7
Compare
b6f1414 to
518ef64
Compare
albertywu
approved these changes
Jun 2, 2026
a7226e7 to
ff9bcc2
Compare
518ef64 to
b96ced6
Compare
ff9bcc2 to
c4455b9
Compare
7d6bf16 to
f612697
Compare
c4455b9 to
6380a1a
Compare
## Summary
### Why?
The build stage needs a vendor-agnostic abstraction for talking to a
Build Runner — one that fits the existing extension family (storage,
queue, conflict) and that does not have to change when the first real
backend lands. Design rationale lives in `doc/rfc/build-runner.md`.
### What?
Introduces the `BuildRunner` contract — three verbs, all keyed by an `entity.BuildID`: `Trigger` submits a build (ordered `base` + `head` change sets plus a free-form metadata map) and returns its ID; `Status` fetches the current `BuildStatus` and runner-defined metadata; `Cancel` requests cancellation. See `extension/buildrunner/build_runner.go` for the signatures.
Highlights:
- `Trigger` takes ordered `base` (assumed-good prefix from dependency
batches) and `head` (changes being verified) — a runner can cache or
short-circuit a prefix it has validated before, and attribute terminal
failures to base vs head via `BuildMetadata`.
- Build identifiers are the typed `entity.BuildID` (a lightweight
`{ID string}` value, also the queue payload envelope) rather than a
bare `string`, so the runner contract and the on-queue messages share
one identifier type and the compiler distinguishes a build ID from
other string IDs.
- `metadata` is a free-form `map[string]string` for caller-supplied
attributes (requester, ticket ID, trace ID) the runner MAY persist or
echo back via `Status`.
- `Trigger` and `Cancel` return promptly; `Status` MAY be lengthy.
- The `BuildStatus` enum is narrowed to `Accepted` / `Running` /
`Succeeded` / `Failed` / `Cancelled` so every backend can map its
native lifecycle without leaking runner-specific stages.
- `BuildMetadata` is added as a free-form map for round-tripping
runner-defined attributes (build URL, duration, SHA, etc.).
Naming: the interface is `BuildRunner` (not `BuildManager` — the
"Manager" suffix doesn't describe what it does, and "Build" alone would
collide cognitively with `entity.Build`). The package is
`extension/buildrunner` to follow the repo convention used by
`mergechecker`, `changeprovider`, `pusher`, `counter`, and `storage`.
Implementation (noop backend, queue `PublishAfter` primitive, controller
wiring, and the poll-driven `buildsignal` loop) lands in the follow-up
PRs stacked on top of this one.
## Test Plan
- ✅ `make test` — entity/build tests pass; mock-only consumers of the
interface compile and pass.
- ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean.
- ✅ `make build` — all targets compile.
## Summary ### Why? Wiring tests for the orchestrator's build stage need a `BuildRunner` implementation, but real backends (BuildKite, Jenkins, etc.) won't land until later. A noop that immediately reports success is also useful as the local default in `make local-start` and as a best-case baseline where every build passes. ### What? Adds `extension/buildrunner/noop` — a `BuildRunner` that does no real work. Every `Trigger` returns a unique `noop-<n>` build ID with no error; every `Status` returns `BuildStatusSucceeded` with no metadata; `Cancel` is a no-op. The atomic counter keeps IDs unique under concurrent use. ## Test Plan - ✅ `make test` — 36 unit tests pass, including the new `extension/buildrunner/noop:noop_test` (Trigger uniqueness, Status, Cancel, interface satisfaction). - ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean. - ✅ `make build` — all targets compile.
6380a1a to
5bc2c0d
Compare
f612697 to
4f0d367
Compare
JamyDev
approved these changes
Jun 3, 2026
behinddwalls
added a commit
that referenced
this pull request
Jun 3, 2026
) ## Summary ### Why? The build stage needs a vendor-agnostic abstraction for talking to a Build Runner — one that fits the existing extension family (storage, queue, conflict) and that does not have to change when the first real backend lands. Design rationale lives in `doc/rfc/build-runner.md`. ### What? Introduces the `BuildRunner` contract — three verbs, all keyed by an `entity.BuildID`: `Trigger` submits a build (ordered `base` + `head` change sets plus a free-form metadata map) and returns its ID; `Status` fetches the current `BuildStatus` and runner-defined metadata; `Cancel` requests cancellation. See `extension/buildrunner/build_runner.go` for the signatures. Highlights: - `Trigger` takes ordered `base` (assumed-good prefix from dependency batches) and `head` (changes being verified) — a runner can cache or short-circuit a prefix it has validated before, and attribute terminal failures to base vs head via `BuildMetadata`. - Build identifiers are the typed `entity.BuildID` (a lightweight `{ID string}` value, also the queue payload envelope) rather than a bare `string`, so the runner contract and the on-queue messages share one identifier type and the compiler distinguishes a build ID from other string IDs. - `metadata` is a free-form `map[string]string` for caller-supplied attributes (requester, ticket ID, trace ID) the runner MAY persist or echo back via `Status`. - `Trigger` and `Cancel` return promptly; `Status` MAY be lengthy. - The `BuildStatus` enum is narrowed to `Accepted` / `Running` / `Succeeded` / `Failed` / `Cancelled` so every backend can map its native lifecycle without leaking runner-specific stages. - `BuildMetadata` is added as a free-form map for round-tripping runner-defined attributes (build URL, duration, SHA, etc.). Naming: the interface is `BuildRunner` (not `BuildManager` — the "Manager" suffix doesn't describe what it does, and "Build" alone would collide cognitively with `entity.Build`). The package is `extension/buildrunner` to follow the repo convention used by `mergechecker`, `changeprovider`, `pusher`, `counter`, and `storage`. Implementation (noop backend, queue `PublishAfter` primitive, controller wiring, and the poll-driven `buildsignal` loop) lands in the follow-up PRs stacked on top of this one. ## Test Plan - ✅ `make test` — entity/build tests pass; mock-only consumers of the interface compile and pass. - ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean. - ✅ `make build` — all targets compile. ## Stack 1. @ #175 1. #179 1. #176 1. #177
behinddwalls
added a commit
that referenced
this pull request
Jun 3, 2026
## Summary ### Why? The orchestrator's build poll loop needs to space `Status` calls out without consuming `retry_count` / DLQ retry slots. `Delivery.Nack` does schedule redelivery after a delay, but it overloads `retry_count` — which the operator relies on to flag real failures. The fix is a separate "postpone this work" primitive, distinct from "this delivery failed, try again", so both signals stay meaningful. ### What? Adds `Publisher.PublishAfter(ctx, topic, msg, delayMs)` to the queue extension: a fresh message inserted into the topic, made visible to subscribers only after `delayMs` from now. Distinct from `Delivery.Nack(requeueAfterMillis)`: - `Nack` is "this delivery failed, try again" — it bumps `retry_count` and eventually trips DLQ. - `PublishAfter` is "postpone this work" — `retry_count` resets to 0, DLQ stays available for true failures. SQL backing in `extension/queue/mysql`: - New `visible_after BIGINT UNSIGNED NOT NULL DEFAULT 0` column on `queue_messages`. Default 0 means immediately visible — back-compat for any existing rows. - `messageStore.InsertDelayed(ctx, topic, messages, visibleAfterMs)` is the underlying primitive. `Insert` is now a thin wrapper that passes `visibleAfterMs = 0`. - `FetchByOffset` gains a `nowMs` parameter and an `AND visible_after <= ?` predicate so subscribers skip rows whose delivery is still deferred. - `MoveToDLQ` writes `visible_after = 0` explicitly: any delay on the original message has already been consumed by the time it failed. The first consumer of `PublishAfter` is the orchestrator's poll-driven `buildsignal` loop, which lands in the stacked PR on top of this one. ## Test Plan - ✅ `bazel test //extension/queue/...` — all pass, including new coverage for `PublishAfter` (positive / zero / negative delay, closed-publisher) and for `FetchByOffset` skipping deferred rows via the new `visible_after` predicate. - ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean. - ✅ `make build` — all targets compile. ## Issues ## Stack 1. #175 1. #179 1. @ #176 1. #177
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why?
Wiring tests for the orchestrator's build stage need a
BuildRunnerimplementation, but real backends (BuildKite, Jenkins, etc.) won't land
until later. A noop that immediately reports success is also useful as
the local default in
make local-startand as a best-case baselinewhere every build passes.
What?
Adds
extension/buildrunner/noop— aBuildRunnerthat does no realwork. Every
Triggerreturns a uniquenoop-<n>build ID with noerror; every
StatusreturnsBuildStatusSucceededwith no metadata;Cancelis a no-op. The atomic counter keeps IDs unique underconcurrent use.
Test Plan
make test— 36 unit tests pass, including the newextension/buildrunner/noop:noop_test(Trigger uniqueness, Status,Cancel, interface satisfaction).
make fmt lint check-tidy check-gazelle check-mocks— clean.make build— all targets compile.Issues
Stack